py/core/info.py-82-94 (1)

82-94: ⚠️ Potential issue | 🟠 Major

Avoid doing this work at import time.

_build_snapshot() shells out to go env GOVERSION while the module is being imported, so a slow, broken, or hijacked go on PATH can stall or fail a plain import core.info. Make _go_version() lazy or add a timeout and exception guard so import stays cheap and predictable.

🛠️ Suggested fix

-    completed = subprocess.run(
-        [go_binary, "env", "GOVERSION"],
-        capture_output=True,
-        check=False,
-        text=True,
-    )
+    try:
+        completed = subprocess.run(
+            [go_binary, "env", "GOVERSION"],
+            capture_output=True,
+            check=False,
+            text=True,
+            timeout=1,
+        )
+    except (OSError, subprocess.SubprocessError):
+        return os.environ.get("GOVERSION", "")

Also applies to: 124-125

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/info.py` around lines 82 - 94, The module currently shells out to
`go` at import via `_go_version()` (and similarly in `_build_snapshot`), which
can block or fail; change `_go_version()` to be lazy and robust by (1) deferring
execution until called (e.g., wrap with a cached lazy call or remove invocation
at import), (2) invoking subprocess.run with a short timeout and wrapped in
try/except to catch TimeoutError, CalledProcessError, and OSError, and (3)
falling back to os.environ.get("GOVERSION", "") on any failure; apply the same
timeout-and-exception-guard logic to the `go` invocation used by
`_build_snapshot()` so imports remain cheap and non-blocking.

py/core/json.py-24-32 (1)

24-32: ⚠️ Potential issue | 🟠 Major

Remove manual UTF-8 decoding to preserve json.loads byte encoding support.

The current implementation decodes bytes as UTF-8 before passing to jsonlib.loads(), which breaks support for JSON byte streams using UTF-16, UTF-32, or other encodings that the stdlib natively handles. Pass bytes directly to jsonlib.loads() instead.

🛠️ Suggested fix

 def loads(value: str | bytes) -> Any:
     """Deserialise JSON text or bytes.

     json.loads('{"name":"corepy"}')
     """

-    if isinstance(value, bytes):
-        value = value.decode("utf-8")
     return jsonlib.loads(value)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/json.py` around lines 24 - 32, The loads function is currently
decoding bytes to UTF-8 before calling jsonlib.loads, which prevents the stdlib
JSON decoder from handling other byte encodings; instead remove the manual
decoding in loads and pass the original value (whether str or bytes) directly to
jsonlib.loads so that jsonlib.loads(value) can detect and handle byte encodings
correctly (update the function body in loads to stop calling .decode and simply
return jsonlib.loads(value)).

RFC.md-403-408 (1)

403-408: ⚠️ Potential issue | 🟠 Major

Fix the uv pip install subdirectory.

The source tree on Lines 371-378 shows pyproject.toml under py/, so #subdirectory=py/core points past the packaging root. As written, this install command is very likely to fail for anyone following the RFC.

🛠️ Minimal doc fix

-uv pip install "core @ git+ssh://git@forge.lthn.ai:2223/core#subdirectory=py/core"
+uv pip install "core @ git+ssh://git@forge.lthn.ai:2223/core#subdirectory=py"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@RFC.md` around lines 403 - 408, The example install command uses the wrong
pip URL subdirectory; update the `uv pip install` example so the `#subdirectory`
points to the packaging root containing pyproject.toml (change
`#subdirectory=py/core` to `#subdirectory=py` in the example command string
`core @ git+ssh://git@forge.lthn.ai:2223/core#subdirectory=...`) so pip can find
the package metadata and install correctly.

py/core/cache.py-135-149 (1)

135-149: ⚠️ Potential issue | 🟠 Major

Treat malformed cache payloads as absent before get() indexes entry["data"].

Right now any JSON object with a numeric expires_at passes validation, even if data is missing. In that case get() raises KeyError instead of falling back to the provided default.

🛠️ Minimal guard

-        if not isinstance(entry, dict):
+        if not isinstance(entry, dict) or "data" not in entry:
             return None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/cache.py` around lines 135 - 149, The _load_entry function currently
returns entries that lack a "data" key, causing get() to raise KeyError; update
_load_entry (the method named _load_entry and the code path that get() uses to
access entry["data"]) to validate that "data" is present (and optionally of an
expected type) before returning the entry, and if "data" is missing or malformed
treat the cache as absent by unlinking the file (target.unlink(missing_ok=True))
and returning None so get() falls back to the provided default.

py/core/cache.py-115-133 (1)

115-133: ⚠️ Potential issue | 🟠 Major

keys() is exposing expired entries as if they still exist.

This path walk never applies the expiry rules from _load_entry(), so stale files remain visible in keys() until some other call happens to touch them. That makes keys() disagree with get() and has() about what is actually present.

🛠️ One straightforward fix

         for target in sorted(self._base_dir.rglob("*.json")):
             if not target.is_file():
                 continue
             key = target.relative_to(self._base_dir).as_posix()[:-5]
             if normalized_prefix and key != normalized_prefix and not key.startswith(f"{normalized_prefix}/"):
                 continue
+            if self._load_entry(key) is None:
+                continue
             keys.append(key)
         return keys

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/cache.py` around lines 115 - 133, The keys() implementation currently
lists all JSON files without applying expiry logic, so update keys() to consult
the same expiry logic used by _load_entry()/get()/has(): for each candidate file
(the loop over self._base_dir.rglob("*.json")), call _load_entry(path) or a
shared helper that checks/loads metadata and returns None for expired entries;
if the call indicates the entry is expired, delete the file (or skip it) and do
not append the key, otherwise append the key as before—this ensures keys(),
get(), and has() are consistent in treating expired entries.

py/core/config.py-145-151 (1)

145-151: ⚠️ Potential issue | 🟠 Major

The functional get wrapper drops the default argument.

Config.get() supports a fallback value, but config.get(...) does not expose it, so the functional surface cannot express config.get(cfg, "missing", 42). That breaks parity between the wrapper API and the underlying handle.

🛠️ Minimal fix

-def get(config_value: Config, key: str) -> Any:
+def get(config_value: Config, key: str, default: Any = None) -> Any:
@@
-    return config_value.get(key)
+    return config_value.get(key, default)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/config.py` around lines 145 - 151, The functional wrapper get
currently drops the fallback argument supported by Config.get; update the
signature of get(config_value: Config, key: str, default: Any = None) -> Any and
forward that default to the underlying call (i.e., call config_value.get(key,
default)); also update the docstring/example to show using the default
parameter. This ensures parity between the wrapper get and Config.get.

bindings/typemap/typemap.go-195-205 (1)

195-205: ⚠️ Potential issue | 🟠 Major

Treat omitted optional errors as nil.

OptionalError still returns expected argument %d when the slot is missing, so bindings that advertise an optional error/value cannot support zero-argument calls. Returning (nil, nil) for index >= len(arguments) keeps the helper aligned with that contract. Based on learnings: Preserve parity between the Go bindings/runtime contract and the Python package surface when changing primitive names, signatures, or import paths.

Suggested fix

 func OptionalError(arguments []any, index int, functionName string) (error, error) {
-	if index >= len(arguments) {
-		return nil, fmt.Errorf("%s expected argument %d", functionName, index)
-	}
-	if arguments[index] == nil {
+	if index >= len(arguments) || arguments[index] == nil {
 		return nil, nil
 	}
 	return ExpectError(arguments, index, functionName)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/typemap/typemap.go` around lines 195 - 205, OptionalError currently
returns an error when the argument slot is missing; change it to treat omitted
optional errors as nil by returning (nil, nil) when index >= len(arguments) so
callers that expect optional error semantics can call with fewer args. In the
OptionalError function, replace the branch that returns fmt.Errorf("%s expected
argument %d", functionName, index) with a simple return nil, nil, and keep the
existing nil-check and fallback to ExpectError(arguments, index, functionName)
for present non-nil values.

bindings/err/err.go-55-80 (1)

55-80: ⚠️ Potential issue | 🟠 Major

wrap cannot actually omit the cause argument.

core.err.wrap("op", "msg") will fail on Line 56 because the operation string is parsed as an error. If the cause is meant to be optional, this needs to detect whether argument 0 is an error/nil and shift the string indices accordingly. Based on learnings: Preserve parity between the Go bindings/runtime contract and the Python package surface when changing primitive names, signatures, or import paths.

Suggested fix

 func wrap(arguments ...any) (any, error) {
-	sourceError, err := typemap.OptionalError(arguments, 0, "core.err.wrap")
-	if err != nil {
-		return nil, err
-	}
-	operation, err := typemap.ExpectString(arguments, 1, "core.err.wrap")
+	var (
+		sourceError error
+		err         error
+		offset      int
+	)
+	if len(arguments) > 0 {
+		if arguments[0] == nil {
+			offset = 1
+		} else if value, ok := arguments[0].(error); ok {
+			sourceError = value
+			offset = 1
+		}
+	}
+	operation, err := typemap.ExpectString(arguments, offset, "core.err.wrap")
 	if err != nil {
 		return nil, err
 	}
-	message, err := typemap.ExpectString(arguments, 2, "core.err.wrap")
+	message, err := typemap.ExpectString(arguments, offset+1, "core.err.wrap")
 	if err != nil {
 		return nil, err
 	}
 
 	code := ""
-	if len(arguments) > 3 {
-		code, err = typemap.ExpectString(arguments, 3, "core.err.wrap")
+	if len(arguments) > offset+2 {
+		code, err = typemap.ExpectString(arguments, offset+2, "core.err.wrap")
 		if err != nil {
 			return nil, err
 		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/err/err.go` around lines 55 - 80, The wrap function incorrectly
assumes argument 0 is the optional cause; update wrap to detect whether the
first argument is an error/nil and shift parsing of operation/message/code
accordingly: use typemap.OptionalError on args[0] and if it returns a non-nil
error value treat it as the cause, otherwise treat args[0] as the operation
string and call typemap.ExpectString on the shifted indices for operation (arg0
or arg1), message, and optional code; finally call core.Wrap or core.WrapCode
with the resolved sourceError, operation, message, and code. Ensure you keep
using typemap.OptionalError, typemap.ExpectString, and return
core.Wrap/core.WrapCode as before.

py/build/build.sh-8-17 (1)

8-17: ⚠️ Potential issue | 🟠 Major

Prerequisite failures should return a non-zero exit code.

The script currently reports success when gopy or the selected Python binary is missing. That can silently skip Tier 2 builds in CI/CD and local scripts.

Suggested fix

 if ! command -v gopy >/dev/null 2>&1; then
-  echo "Tier 2 build requires gopy + CPython 3.13+ -- see README.md"
-  exit 0
+  echo "Tier 2 build requires gopy + CPython 3.13+ -- see README.md" >&2
+  exit 1
 fi
 
 python_bin="${PYTHON:-python3.13}"
 if ! command -v "${python_bin}" >/dev/null 2>&1; then
-  echo "Tier 2 build requires gopy + CPython 3.13+ -- see README.md"
-  exit 0
+  echo "Tier 2 build requires gopy + CPython 3.13+ -- see README.md" >&2
+  exit 1
 fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/build/build.sh` around lines 8 - 17, The current prereq checks in build.sh
use exit 0 which signals success; change those to non-zero exits so failures
propagate: update both checks that test for gopy (command -v gopy) and the
python binary referenced by python_bin (python_bin="${PYTHON:-python3.13}" and
command -v "${python_bin}") to call exit 1 (or another non-zero code) instead of
exit 0 so CI/local callers detect the missing prerequisites.

bindings/crypto/crypto.go-101-113 (1)

101-113: ⚠️ Potential issue | 🟠 Major

random_bytes needs an upper bound on requested size.

A caller-controlled size is used for direct allocation. Very large values can cause OOM conditions and take down the runtime process.

Suggested fix

+const maxRandomBytes = 1 << 20 // 1 MiB; tune to the runtime contract.
+
 func randomBytes(arguments ...any) (any, error) {
 	size, err := typemap.ExpectInt(arguments, 0, "core.crypto.random_bytes")
 	if err != nil {
 		return nil, err
 	}
 	if size < 0 {
 		return nil, fmt.Errorf("core.crypto.random_bytes expected a non-negative size")
 	}
+	if size > maxRandomBytes {
+		return nil, fmt.Errorf("core.crypto.random_bytes size exceeds maximum of %d bytes", maxRandomBytes)
+	}
 	buffer := make([]byte, size)
 	if _, err := cryptorand.Read(buffer); err != nil {
 		return nil, fmt.Errorf("core.crypto.random_bytes failed to read randomness: %w", err)
 	}
 	return buffer, nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/crypto/crypto.go` around lines 101 - 113, The randomBytes function
currently allocates a caller-controlled number of bytes which can cause OOM; add
an explicit upper bound constant (e.g. MaxRandomBytes) and validate size <=
MaxRandomBytes after the existing non-negative check, returning a clear error if
exceeded; only proceed to make([]byte, size) and call cryptorand.Read(buffer)
when size is within the allowed limit so huge allocations are prevented (update
references in randomBytes to use the constant and the new validation).

bindings/service/service.go-81-100 (1)

81-100: ⚠️ Potential issue | 🟠 Major

Do not use context.Background() for lifecycle calls.

These operations can block indefinitely and cannot be cancelled or timed out, so a hung service will stall the interpreter. Thread through a request-scoped context from the runtime, or apply a bounded timeout here if no caller context exists.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/service/service.go` around lines 81 - 100, startAll and stopAll
currently call instance.ServiceStartup(context.Background(), ...) and
instance.ServiceShutdown(context.Background()) which uses an uncancelable global
context; instead accept or derive a request-scoped context and/or apply a
bounded timeout. Modify startAll and stopAll to obtain a context from the
runtime/request (e.g., read a context argument or use an existing typemap
helper) and pass that to ServiceStartup and ServiceShutdown, or if no caller
context is available create a cancellable context with a sensible timeout via
context.WithTimeout and defer cancel() before calling the methods; ensure you
use the same context variable for both ServiceStartup and ServiceShutdown calls
in startAll/stopAll respectively and include cancellation handling.

bindings/i18n/i18n.go-116-134 (1)

116-134: ⚠️ Potential issue | 🟠 Major

Only persist the locale after the translator accepts it.

setLanguage writes item.locale before validating the new language with the translator. If the translator rejects the language, the handle now reports a locale that was never accepted.

🔧 Suggested fix

-	item.locale = lang
-	if item.translator != nil {
-		if err := item.translator.SetLanguage(lang); err != nil {
-			return nil, err
-		}
-	}
+	if item.translator != nil {
+		if err := item.translator.SetLanguage(lang); err != nil {
+			return nil, err
+		}
+	}
+	item.locale = lang

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/i18n/i18n.go` around lines 116 - 134, The function setLanguage
currently assigns item.locale before validating with item.translator, so move
the assignment until after translator validation: call
item.translator.SetLanguage(lang) first (only when item.translator != nil),
returning the error if it fails, and then set item.locale = lang; if
item.translator is nil simply set item.locale = lang; ensure item is returned
unchanged on error. This change targets the setLanguage function and the calls
to item.translator.SetLanguage(lang).

bindings/i18n/i18n.go-68-87 (1)

68-87: ⚠️ Potential issue | 🟠 Major

Make translator swaps atomic.

setTranslator stores the new translator before SetLanguage succeeds. If that call fails, the handle is left half-updated and subsequent calls will see the new translator even though the operation returned an error.

🔧 Suggested fix

-	item.translator = translatorValue
-	if item.locale != "" {
-		if err := item.translator.SetLanguage(item.locale); err != nil {
-			return nil, err
-		}
-	}
+	if item.locale != "" {
+		if err := translatorValue.SetLanguage(item.locale); err != nil {
+			return nil, err
+		}
+	}
+	item.translator = translatorValue

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/i18n/i18n.go` around lines 68 - 87, The setTranslator function
currently assigns item.translator before calling translator.SetLanguage, leaving
the handle partially updated if SetLanguage fails; change the logic in
setTranslator (and use the translator type/value) to first validate and call
SetLanguage on the new translator when item.locale != "" and only assign
item.translator = translatorValue after SetLanguage succeeds (or alternatively
perform the assignment and revert on error), ensuring the swap is atomic and any
error leaves item.translator unchanged; locate references to setTranslator,
item.translator, translator.SetLanguage, item.locale and expectHandle to
implement this change.

runtime/gpython_smoke_test.go-27-45 (1)

27-45: ⚠️ Potential issue | 🟠 Major

Quote the go binary path before embedding it.

Interpolating goBinary directly into the Python snippet makes this smoke test brittle on paths with backslashes or quotes, so it can fail on otherwise valid environments.

🔧 Suggested fix

- print(subprocess.check_output(["` + goBinary + `", "env", "GOOS"]))
+ print(subprocess.check_output([` + strconv.Quote(goBinary) + `, "env", "GOOS"]))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@runtime/gpython_smoke_test.go` around lines 27 - 45, The embedded goBinary
path is injected raw into the Python snippet (see variable goBinary and the
interpreter.Run call), which breaks on paths with quotes or backslashes; fix by
properly quoting/escaping the path before embedding—e.g. replace the inline
concat with a quoted form using strconv.Quote or fmt.Sprintf("%q", goBinary)
when constructing the Python string so the path is safely wrapped/escaped inside
the triple-quoted snippet passed to interpreter.Run.

bindings/config/config.go-92-131 (1)

92-131: ⚠️ Potential issue | 🟠 Major

Do not silently coerce invalid env fallbacks.

intValue and boolValue currently swallow strconv.Atoi / strconv.ParseBool failures and return 0 / false. That makes a misconfigured environment indistinguishable from an unset key and can hide configuration mistakes.

♻️ Suggested fix

 func intValue(arguments ...any) (any, error) {
@@
     if value, ok := lookupEnvironment(key); ok {
         parsed, err := strconv.Atoi(value)
-        if err == nil {
-            return parsed, nil
+        if err != nil {
+            return nil, err
         }
+        return parsed, nil
     }
     return 0, nil
 }
@@
     if value, ok := lookupEnvironment(key); ok {
         parsed, err := strconv.ParseBool(value)
-        if err == nil {
-            return parsed, nil
+        if err != nil {
+            return nil, err
         }
+        return parsed, nil
     }
     return false, nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/config/config.go` around lines 92 - 131, The intValue and boolValue
helpers currently ignore strconv.Atoi/ParseBool errors and return zero values,
hiding invalid environment fallbacks; update intValue and boolValue so that when
lookupEnvironment(key) returns a string but parsing fails you return the parsing
error (wrapped or returned directly) instead of silently returning 0/false,
preserving existing behavior where config.Get(key).OK short-circuits to the
config value and keeping typemap.Expect* errors unchanged; reference the
intValue and boolValue functions and ensure the returned error includes the
original strconv error message for easier debugging.

py/core/crypto.py-16-22 (1)

16-22: ⚠️ Potential issue | 🟠 Major

SHA-1 is intentionally exported in the public API; consider whether this should be isolated or removed.

sha1 is explicitly listed in crypto.__all__ and therefore part of the public surface. Since SHA-1 is cryptographically broken, exposing it without explicit guidance risks misuse. The function is only tested (py/tests/test_core.py:222) and has no production consumers. If SHA-1 is not required for backward compatibility or documented interoperability, either remove it or relocate it to a legacy or internal namespace with explicit "non-security" documentation to discourage accidental misuse.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/crypto.py` around lines 16 - 22, The sha1 function is exposed in the
public API via crypto.__all__, which risks accidental use of broken SHA-1;
either remove it from the public surface or move it to a legacy/internal
namespace and update references/tests accordingly. Specifically, either (A)
remove "sha1" from crypto.__all__ and adjust py/tests/test_core.py to import/use
the internal test helper or update the test to use a non-crypto utility, or (B)
relocate the function sha1 into a new internal module (e.g., crypto_legacy or
_crypto_internal), add clear docstring noting it is non-secure/legacy, export
only for tests, and update imports in py/tests/test_core.py to reference the new
module; ensure no other production modules import crypto.sha1.

bindings/cache/cache.go-314-316 (1)

314-316: ⚠️ Potential issue | 🟠 Major

Do not swallow JSON decode failures in get.

Line 315 currently treats malformed cache content as “not found”. This masks data corruption and silently changes behaviour.

Proposed fix

 	if err := decoder.Decode(&decoded); err != nil {
-		return nil, false, nil
+		return nil, false, fmt.Errorf("core.cache.get failed to decode entry %q: %w", path, err)
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/cache/cache.go` around lines 314 - 316, The get function currently
swallows JSON decoding errors (the decoder.Decode(&decoded) failure) and returns
(nil, false, nil) which hides corruption; change the error path to propagate the
decoding error instead of treating it as “not found”: when
decoder.Decode(&decoded) returns an error, return nil, false, err (or wrap the
error with context) so callers can detect and handle malformed cache entries;
update any callers/tests that expect the old silent behavior if necessary.

py/core/strings.py-66-85 (1)

66-85: ⚠️ Potential issue | 🟠 Major

split and split_n require test coverage and explicit handling for empty separators.

Lines 72 and 85 delegate directly to Python's str.split(), which raises ValueError on empty separators. There is no test coverage for the empty separator case (test_core.py only covers non-empty: "key=value=extra", "=", 2). If the Go-side core.Split implementation handles empty separators, the Python surface will diverge. Add:

• Test cases covering empty separator inputs
• Explicit error handling or documented behaviour for separator == ""

This ensures parity with the Go bindings and prevents surprises at the API boundary.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/strings.py` around lines 66 - 85, split and split_n currently
delegate to str.split and will raise a ValueError on an empty separator without
tests; update both functions (split and split_n) to explicitly handle separator
== "" by mirroring the Go/core.Split behavior (inspect core.Split and either
raise a clear ValueError with a descriptive message or implement the same
empty-separator semantics), add unit tests in test_core.py covering
empty-separator inputs for both split and split_n (including limit edge-cases
like 0 and negative), and document the chosen behavior in the function
docstrings.

runtime/interpreter_test.go-63-78 (1)

63-78: ⚠️ Potential issue | 🟠 Major

Ensure interpreters are always closed in the shared test helper.

newTestInterpreter returns live interpreters without t.Cleanup, so resources remain open across many tests.

💡 Suggested fix

 func newTestInterpreter(t *testing.T) testInterpreter {
 	t.Helper()
@@
 	caller, ok := interpreter.(testInterpreter)
 	if !ok {
+		_ = interpreter.Close()
 		t.Fatalf("interpreter does not expose direct calls: %T", interpreter)
 	}
+	t.Cleanup(func() {
+		_ = caller.Close()
+	})
 	return caller
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@runtime/interpreter_test.go` around lines 63 - 78, newTestInterpreter returns
a live interpreter and doesn't register a test cleanup, leaking resources;
update newTestInterpreter to call t.Cleanup to close the interpreter when the
test finishes (invoke the interpreter's shutdown method — e.g.
interpreter.Close() or the appropriate teardown on the testInterpreter
interface) after successful creation and module registration so the interpreter
returned by newTestInterpreter is always closed automatically; reference
newTestInterpreter, testInterpreter, corepyruntime.New and
register.DefaultModules when making the change.

bindings/stdlib/stdlib.go-294-315 (1)

294-315: ⚠️ Potential issue | 🟠 Major

Capture stderr explicitly in runCommand to reliably return stderr output.

The current implementation uses cmd.Output() without setting cmd.Stderr, which means the returned stderr field will always be empty even when the command writes to stderr. The exec.ExitError.Stderr field is only populated if cmd.Stderr is explicitly configured before execution.

Suggested fix

 import (
+	"bytes"
 	"crypto/sha1"
 	"crypto/sha256"
 	"encoding/base64"
 	"encoding/hex"
 	"fmt"
 	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"slices"
 	"strings"

 func runCommand(functionName string, arguments []any, check bool) (map[string]any, error) {
 	commandLine, err := commandLineArguments(arguments, functionName)
 	if err != nil {
 		return nil, err
 	}
 	cmd := exec.Command(commandLine[0], commandLine[1:]...)
-	output, err := cmd.Output()
-	stderr := ""
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	err := cmd.Run()
 	exitCode := 0
 	if err != nil {
 		exitCode = -1
 		if exitErr, ok := err.(*exec.ExitError); ok {
 			exitCode = exitErr.ExitCode()
 		}
 		if check {
+			stderrText := stderr.String()
-			if stderr != "" {
-				return nil, fmt.Errorf("%s exited with status %d: %s", functionName, exitCode, firstLine(stderr))
+			if stderrText != "" {
+				return nil, fmt.Errorf("%s exited with status %d: %s", functionName, exitCode, firstLine(stderrText))
 			}
 			return nil, fmt.Errorf("%s exited with status %d: %w", functionName, exitCode, err)
 		}
 	}
 	return map[string]any{
 		"args":        commandLine,
-		"stdout":      string(output),
-		"stderr":      stderr,
+		"stdout":      stdout.String(),
+		"stderr":      stderr.String(),
 		"returncode":  exitCode,
 		"timed_out":   false,
 		"ok":          exitCode == 0,
 		"core_shadow": true,
 	}, nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@bindings/stdlib/stdlib.go` around lines 294 - 315, The runCommand
implementation uses cmd.Output() but never wires cmd.Stderr, so stderr is always
empty; fix runCommand by creating a bytes.Buffer (e.g., stderrBuf) and setting
cmd.Stderr = &stderrBuf before calling cmd.Output(), then use stderrBuf.String()
as the stderr return value (and, when err is an *exec.ExitError, prefer
exitErr.Stderr if populated but fall back to stderrBuf.String()); ensure you
still set exitCode from exitErr.ExitCode() and return the captured stderr in the
returned map (referencing cmd, cmd.Output, cmd.Stderr, stderrBuf, and exitErr).

py/core/task.py-138-150 (1)

138-150: ⚠️ Potential issue | 🟠 Major

Fix A002 builtin shadowing in new_step while preserving backwards compatibility with input= callers.

The input parameter is flagged as A002 (shadows Python builtin) and will fail ruff linting, which is enabled in the repository configuration. The parameter is actively used by the Go runtime bindings (task.new_step("consume", input="previous")), so renaming without compatibility handling would break the API contract. Accept input via **kwargs with an internal non-shadowing name to preserve the public API whilst passing linting.

Suggested fix (maintains backwards compatibility)

 def new_step(
     action: str,
     with_values: dict[str, Any] | None = None,
     async_step: bool = False,
-    input: str = "",
+    input_value: str = "",
+    **kwargs: Any,
 ) -> Step:
     """Create a Step value.
 
     task.new_step("echo", {"text": "hello"})
     """
 
-    return Step(action=action, with_values={} if with_values is None else dict(with_values), async_step=async_step, input=input)
+    if "input" in kwargs:
+        input_value = str(kwargs.pop("input"))
+    if kwargs:
+        unexpected = ", ".join(sorted(kwargs))
+        raise TypeError(f"task.new_step got unexpected keyword argument(s): {unexpected}")
+    return Step(
+        action=action,
+        with_values={} if with_values is None else dict(with_values),
+        async_step=async_step,
+        input=input_value,
+    )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/task.py` around lines 138 - 150, Rename the internal parameter to
avoid shadowing by removing the explicit input parameter from new_step's
signature and accept **kwargs instead; inside new_step, extract the public
"input" kwarg (e.g., input_value = kwargs.pop("input", "")) and use that
internal name when constructing the Step, preserving the default "" and
supporting callers that pass input="..."; also validate that no unexpected
kwargs remain (raise TypeError if kwargs not empty) so other callers get a clear
error, and keep with_values handling and async_step unchanged—this preserves the
public input= API used by Go bindings while fixing the A002 builtin shadowing
lint.

py/core/data.py-117-123 (1)

117-123: ⚠️ Potential issue | 🟠 Major

Block path traversal when resolving mounted paths.

At Line 117-123, _resolve permits .. segments (for example, fixtures/../secret.txt) to escape the mounted root. This impacts read_file, read_string, list, and extract.

Proposed fix

 def _resolve(self, logical_path: str) -> Path:
     mount_name, _, relative_path = logical_path.partition("/")
     if mount_name not in self._mounts:
         raise KeyError(f"mount not found: {mount_name}")
-    root = self._mounts[mount_name]
-    return root if relative_path == "" else root / relative_path
+    root = self._mounts[mount_name].resolve()
+    candidate = root if relative_path == "" else (root / relative_path).resolve()
+    if candidate != root and root not in candidate.parents:
+        raise ValueError(f"path escapes mount root: {logical_path}")
+    return candidate

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/data.py` around lines 117 - 123, The _resolve method currently allows
path traversal via '..'; fix by normalizing and resolving the combined path and
verifying it stays under the mount root: after obtaining root from self._mounts
and computing relative_path, compute root_resolved = root.resolve() and
candidate = (root / relative_path).resolve() (use strict=False if needed), then
if candidate is not inside root_resolved (use Path.is_relative_to(root_resolved)
or compare root_resolved in candidate.parents) raise a KeyError/ValueError
indicating forbidden path traversal; return root_resolved for empty
relative_path or candidate otherwise. Ensure this change is applied in the
_resolve function so read_file/read_string/list/extract use the safe path.

py/core/medium.py-186-195 (1)

186-195: ⚠️ Potential issue | 🟠 Major

Do not clobber the text field for non-UTF-8 bytes.

For binary payloads, writing "" into self.payload_field makes read_text() return an empty string instead of exposing that the value is not UTF-8. Please omit the text field when decoding fails so the bytes path remains authoritative.

🔧 Proposed fix

     def write_bytes(self, value: bytes) -> bytes:
         """Write bytes into a Qdrant point payload."""
 
         payload = {self.bytes_field: base64.b64encode(value).decode("ascii")}
         try:
             payload[self.payload_field] = value.decode("utf-8")
         except UnicodeDecodeError:
-            payload[self.payload_field] = ""
+            pass
         self._set_payload(payload)
         return value

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/medium.py` around lines 186 - 195, In write_bytes, avoid clobbering
the text field when binary data isn't UTF-8: in the write_bytes method
(referencing self.bytes_field, self.payload_field and _set_payload), build
payload with the base64-encoded bytes under self.bytes_field and only set
self.payload_field to the decoded string when value.decode("utf-8") succeeds; if
a UnicodeDecodeError occurs, do not add self.payload_field to payload (omit it)
so the bytes path remains authoritative before calling
self._set_payload(payload) and returning value.

py/core/dns.py (1)

50-59: Tighten _unique to an iterable type.

The implementation iterates over values, but the signature allows plain object, which is not iterable from a type-checker’s point of view. Please annotate this as an Iterable[...] so the helper stays consistent with the typed Python surface. As per coding guidelines: py/core/**/*.py: Keep the Python package typed and consistent with the pyproject.toml target of Python 3.12+.

🛠️ Suggested typing fix

+from collections.abc import Iterable
 import socket
@@
-def _unique(values: list[str] | tuple[str, ...] | object) -> list[str]:
+def _unique(values: Iterable[object]) -> list[str]:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/dns.py` around lines 50 - 59, The _unique function's parameter is
currently typed as list[str] | tuple[str, ...] | object which allows
non-iterable types; change the annotation to accept an iterable (e.g., values:
Iterable[str] | Iterable[object] or simply Iterable[object] depending on desired
input), import Iterable from typing, and keep the function body unchanged so
static type-checkers know values is iterable; update the signature for the
_unique function accordingly and ensure any callers still match the new Iterable
typing.

py/core/__init__.py (2)

15-21: Broaden echo() so it preserves the caller's type.

This is an identity helper, so typing it as str narrows the public API unnecessarily and makes valid non-string uses fail static checking.

Suggested change

+from typing import TypeVar
+
+T = TypeVar("T")
+
-def echo(value: str) -> str:
+def echo(value: T) -> T:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/__init__.py` around lines 15 - 21, The echo function is currently
typed to str which unnecessarily narrows its API; change it to be generic using
a TypeVar (e.g., T = TypeVar("T")) and annotate the function as def echo(value:
T) -> T so it preserves the caller's type. Add the TypeVar import from typing
(or typing_extensions if used elsewhere) and update the echo signature and
docstring example if needed to reflect generic behavior; keep the implementation
returning value unchanged and ensure no other behavior changes.

---

24-56: Sort __all__ to keep the export list lint-clean.

Ruff already flags this, and sorting the export list will keep the module surface deterministic.

Suggested change

 __all__ = [
-    "array",
     "action",
     "agent",
     "api",
+    "array",
     "cache",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/__init__.py` around lines 24 - 56, The exported names in the __all__
list are unsorted and trigger linter warnings; sort the entries in the __all__
list (the "__all__" symbol) alphabetically (case-insensitive) while preserving
the existing string quotes, commas, and list structure so the module export
order is deterministic and Ruff warnings are resolved.

py/core/task.py (1)

236-240: Add exception observability to async step execution.

Catching Exception and silently returning in the daemon thread prevents any visibility into async step failures. This makes debugging difficult and hides real problems from operators and test runners. Consider logging the exception or using an exception handler callback to surface failures.

def _run_async(current_action: action_module.Action, step_values: dict[str, Any], context: Any) -> None:
    try:
        current_action.run(step_values, context)
    except Exception:
        return

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@py/core/task.py` around lines 236 - 240, The _run_async function currently
swallows all exceptions from current_action.run, losing observability; change
the except block to "except Exception as e:" and surface the error by logging
the exception (e.g., use logging.getLogger(__name__).exception(...) or an
existing module logger) including identifying info like current_action (or
current_action.name) and step_values, and if the provided context exposes an
error callback (e.g., context.on_error or similar), invoke it with the exception
and current_action so callers/tests can react; ensure you import logging if
needed and avoid re-raising so the daemon thread stays alive.

runtime/bootstrap/interpreter.go (1)

550-554: Fix stale doc comment above splitTopLevel.

The comment currently describes SplitKeywordArguments, which does not match the function below and can mislead future maintenance.

Suggested doc fix

-// SplitKeywordArguments separates positional arguments from a trailing
-// KeywordArguments payload.
-//
-//	positional, keywordArguments := runtime.SplitKeywordArguments(arguments)
+// splitTopLevel splits by `separator` while respecting nested groupings
+// and quoted string literals.
 func splitTopLevel(value string, separator rune) ([]string, error) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@runtime/bootstrap/interpreter.go` around lines 550 - 554, The doc comment
above splitTopLevel is stale (mentions SplitKeywordArguments); update it to
accurately describe splitTopLevel: explain that splitTopLevel splits the input
string by the given separator rune at top-level (i.e., it does not split when
the separator is inside nested structures or quoted segments) and state the
returned values (a slice of top-level segments and an error). Place this
corrected comment immediately above the splitTopLevel function declaration and
ensure it follows Go doc comment style (starts with the function name).